Conversation
There was a problem hiding this comment.
Pull request overview
Refactors insights project repository selection to stop relying on insightsProjects.repositories and instead use public.repositories.enabled as the source of truth for which repos are active.
Changes:
- Adds
enabledto repository-fetching/grouping responses and threads it through DAL → API → UI. - Updates insights project update flow to sync
public.repositories.enabled(and avoids writinginsightsProjects.repositories). - Removes deprecated GitHub repo sync activity/service code and related backend wiring.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| services/libs/data-access-layer/src/utils.ts | Makes updateTableById resilient when there are no updatable fields (still updates updatedAt). |
| services/libs/data-access-layer/src/segments/index.ts | Extends repo grouping query/return type to include enabled. |
| services/libs/data-access-layer/src/repositories/index.ts | Updates doc comment for syncRepositoriesEnabledStatus to reflect new usage. |
| services/libs/data-access-layer/src/integrations/index.ts | Propagates enabled through findRepositoriesForSegment and updates return type. |
| services/libs/data-access-layer/src/collections/index.ts | Stops updating insightsProjects.repositories; syncs public.repositories.enabled when repositories is provided. |
| services/libs/common_services/src/services/integration.service.ts | Removes deprecated syncGithubRepositoriesToInsights flow. |
| services/apps/nango_worker/src/workflows/syncGithubIntegration.ts | Removes post-sync activity call for deprecated insights project repo syncing. |
| services/apps/nango_worker/src/activities/nangoActivities.ts | Removes deprecated activity and unused service import. |
| services/apps/nango_worker/src/activities.ts | Removes export for deleted activity. |
| frontend/src/modules/admin/modules/insights-projects/insight-project-helper.ts | Updates form/repo utilities to rely on backend-provided enabled instead of insightsProjects.repositories. |
| backend/src/services/integrationService.ts | Removes insights-project repository syncing and stops writing data.repositories during integration updates. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for (const [platform, repos] of Object.entries(reposByPlatform)) { | ||
| result[platform] = repos.map((repo) => ({ | ||
| url: normalizeRepoUrl(repo.url), | ||
| label: extractLabelFromUrl(repo.url), | ||
| enabled: repo.enabled, |
There was a problem hiding this comment.
findRepositoriesForSegment returns url: normalizeRepoUrl(repo.url), but the url value is later used as the identifier the UI sends back when updating an insights project’s enabled repos. Since normalizeRepoUrl can change the string (e.g., http→https, remove www., strip .git/trailing slashes), the incoming enabled URL list may no longer exactly match public.repositories.url, causing syncRepositoriesEnabledStatus to disable repos unexpectedly. Consider returning the raw stored repo.url as the identifier (and only normalize for label/display), or ensure URLs are normalized at write-time so stored and returned values round-trip consistently.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| qx: QueryExecutor, | ||
| segmentId: string, | ||
| ): Promise<Record<string, Array<{ url: string; label: string }>>> { | ||
| ): Promise<Record<string, Array<{ url: string; label: string; enabled: boolean }>>> { |
There was a problem hiding this comment.
Unused exported function after refactoring
Low Severity
The normalizeRepoUrl function is exported but no longer used anywhere in the codebase. This PR removed its only usage in findRepositoriesForSegment (which previously called normalizeRepoUrl(url) but now uses repo.url directly), leaving the function as dead code. The function can be safely removed unless it's intended for future use.
| insightsProjectId, | ||
| isFirstUpdate: true, | ||
| platform, | ||
| repositories, |
There was a problem hiding this comment.
@mbani01 just to double check, we don't need to re-update the insightsProject anymore here ? the data is already synced?
There was a problem hiding this comment.
No we don't need to re-update repositories, since the enabled is field in public.repositories is by default set to true
mbani01
deleted the
chore/full_deprecation_of_insightsProjectRepositories
branch
2 participants
This pull request refactors the way repository data is managed and synchronized between backend and frontend, focusing on removing the direct use of the
insightsProject.repositoriesfield and instead relying on therepositories.enabledstatus for repository selection. The changes streamline repository management, improve data consistency, and simplify the update flow for insights projects.Backend refactoring and data model changes:
repositoriesfield oninsightsProjectobjects, including the removal of thesyncGithubRepositoriesToInsightsmethod and related update flows inIntegrationService. [1] [2] [3] [4] [5]enabledboolean flag for each repo, reflecting whether the repository is active for the project. This affects the return types and data structures ingetReposBySegmentGroupedByPlatform,findRepositoriesForSegment, and related interfaces. [1] [2] [3] [4] [5]repositories.enabledfield in thepublic.repositoriestable, rather than updating a list of URLs on the insights project itself. [1] [2]Frontend adjustments:
repositoriesfield.enabledflag in the repository objects, ensuring that the UI reflects the actual enablement status from the backend. [1] [2]Code cleanup and removal of deprecated logic:
syncGithubReposToInsightsactivity and its references across the codebase. [1] [2] [3] [4]These changes collectively ensure that repository enablement is now consistently managed via the
enabledfield in therepositoriestable, improving the reliability and clarity of repository management in both backend and frontend systems.Note
Medium Risk
This changes how repo selection is persisted (moving from an InsightsProject URL list to
public.repositories.enabled) and removes an existing GitHub-to-insights sync path, which could affect which repos appear enabled if any callers still rely on the deprecated field.Overview
Stops treating
insightsProjects.repositoriesas the source of truth for repo selection and instead relies onpublic.repositories.enabled.Backend updates remove repo-list syncing during integration create/update and delete the
CommonIntegrationService.syncGithubRepositoriesToInsights+ related Nango worker activity/workflow calls;updateInsightsProjectno longer writes the repositories list, and the DALupdateInsightsProjectignores DB updates to therepositoriescolumn while still using an optionalrepositoriesinput to togglepublic.repositories.enabled.Repository lookup APIs now return
{url,label,enabled}(including a newenabledfield propagated from SQL), and the admin UI helpers are simplified to consume/preserve thatenabledflag when building and merging repository lists.Written by Cursor Bugbot for commit 3fb69ee. This will update automatically on new commits. Configure here.